Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for 3-way comparison in tests_to_html.py #1416

Conversation

krohmerNV
Copy link
Contributor

Tried to do a comparison between MDL 1.6, 1.7, and 1.8 and found two issues:

  • the computed file paths are not correct when passing different --inputdir1, --inputdir2, and --inputdir3
  • when comparing basically mdl against mdl and mdl, the file names of the diff images collide

@jstone-lucasfilm
Copy link
Member

@krohmerNV I like the idea of this improvement, though in my local tests it seems to break the simpler two-way comparisons between GLSL and OSL:

TwoWayComparison

@krohmerNV
Copy link
Contributor Author

I'll investigate, thanks!

@jstone-lucasfilm
Copy link
Member

@krohmerNV Just bumping this thread, in case you have a chance to follow up on this proposal!

@krohmerNV
Copy link
Contributor Author

thanks @jstone-lucasfilm, I need to make time for this in the next week

@krohmerNV
Copy link
Contributor Author

krohmerNV commented Aug 16, 2024

@jstone-lucasfilm updated the default parameters of the script to behave like before.
Sorry it took so long.

Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these improvements, @krohmerNV!

@jstone-lucasfilm
Copy link
Member

Just as external validation of the PR, here are the latest GLSL/OSL render comparisons with these changes in place:

MaterialXRenderTests_08_24_2024.pdf

@jstone-lucasfilm jstone-lucasfilm changed the title Fixes for the 3-way comparison in tests_to_html.py Fixes for 3-way comparison in tests_to_html.py Aug 24, 2024
@jstone-lucasfilm jstone-lucasfilm merged commit e1c8cc2 into AcademySoftwareFoundation:main Aug 24, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants